Skip to content

Verify and fix type hints, annotations, and docstrings - #1268

Merged
bact merged 7 commits into
devfrom
copilot/verify-type-hints-and-docs
Feb 3, 2026
Merged

Verify and fix type hints, annotations, and docstrings#1268
bact merged 7 commits into
devfrom
copilot/verify-type-hints-and-docs

Conversation

Copilot AI commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

What do these changes do

Eliminates all 84 mypy type checking errors across 191 source files and ensures type hints match docstrings throughout the codebase.

What was wrong

Recent PRs (#1262-#1267) added type hints that contained errors:

  • Type mismatches: Functions returned Any instead of declared types (38 instances)
  • Unreachable code: Non-optional parameters checked for None (14 instances)
  • Missing annotations: Optional[str] needed where None was possible
  • Import issues: Module/function name collisions (pos_tag)
  • Inconsistent patterns: Mixed use of List[str] vs list[str], missing Union[] wrapper

How this fixes it

Core type system fixes:

  • Added proper Union types for polymorphic model classes (translate/core.py)
  • Fixed return type inference with explicit type annotations on decoded lists
  • Made parameters Optional[str] where None checks existed
  • Added TYPE_CHECKING imports to prevent circular dependencies

External library boundaries:

  • Added # type: ignore[no-any-return] for untyped external calls (transformers, numpy, fairseq)
  • Proper error handling for corpus paths that may return None

Import organization:

  • Resolved pos_tag collision: from pythainlp.tag.pos_tag import pos_tag
  • Moved ufal imports to function scope for lazy loading
  • Added ufal.* to mypy ignore list in pyproject.toml

Type narrowing:

  • Used cast() for sent_tokenize that returns Union[list[str], list[list[str]]] based on input type
  • Added runtime None checks before passing to non-optional parameters

Standards compliance:

  • Native types (list, dict) instead of List, Dict for Python 3.9+
  • Union[A, B] instead of A | B for compatibility
  • Specific error codes on all type:ignore comments

Your checklist for this pull request

  • Passed code styles and structures
  • Passed code linting checks and unit test
Original prompt

Verify type hints and annotations and docstring.

Current codebase contains some type hints and comments for static type checkers,
including ones that added by these PRs:

#1262
#1263
#1264
#1265
#1266
#1267

Verify if the annotations are correct, and if the docstring and other documentation are up to date to the type hints.

Goals

  • When a type hint exist, it is correct and the docstring is correct
  • Use "ignore" comments only when it is necessary and with a scoped/specific type

Instructions

  • Follow best practices and standard Python type hint patterns.
  • Use mypy as main assistant.
    • mypy is already in "dev" extra dependencies in pyproject.toml
    • mypy configuration is in pyproject.toml
    • Sometimes mypy may report errors wrongly due to cache issues. Try to reset the cache.
  • Use pyright, pyrefly, and pytype for second opinions.
  • Required dependencies for each test suite are in pyproject.toml. Install them to avoid errors. See https://github.com/PyThaiNLP/pythainlp/blob/dev/tests/README.md
  • Use native type in type hint (list, tuple, .. instead of List, Tuple, ..)
  • Do not use A | B style Union, use full Union[] instead.
  • We want to keep compatibility with Python 3.9 strict and allow type inspection tools to work correctly.
  • When insert typing imports, put it in appropriate location and order
  • Use "if TYPE_CHECKING import" block when possible.
  • When reordering the imports, beware of circular import. Read comments near imports to get more information.
  • Remove unused imports
  • Do not allow the use of assert in production code (it is only allowed for testing and debugging).
  • Try to fix the type, the scope logic, or do the casting, before putting #type: ignore comment.
  • When dealing with types from external libraries:
    • Check if type information is provided (maybe through stubs). If it is, cast wrapper may not necessary.
    • If return type is not provided, may be it is appropriate to use # type: ignore[no-any-return] annotation for clarity.
  • Recheck necessity when do casting.
  • Recheck necessity of # noqa: and # type: ignore
  • Provide type stubs as necessary.
  • After changes for type hints, recheck for functionality correctness.
  • Recheck docstring and documentation consistency with the code; They should match the updated type hints.
  • Recheck formatting with ruff
  • Do another code review
  • Address code review feedback

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI assigned Copilot and bact Feb 3, 2026
@bact bact added the refactoring a technical improvement which does not add any new features or change existing features. label Feb 3, 2026
@bact bact added this to the 5.3 milestone Feb 3, 2026
Copilot AI and others added 6 commits February 3, 2026 11:21
- Fixed translate/core.py to properly type the model Union
- Fixed zh_th.py, th_fr.py, small100.py to return str properly
- Fixed tokenization_small100.py prefix_tokens to Optional[list[int]]
- Fixed en_th.py, word2word_translate.py with type ignore comments
- Fixed transliterate files (umt5_thaig2p, thaig2p_v2, w2p)
- Reduced mypy errors from 84 to 67

Co-authored-by: bact <128572+bact@users.noreply.github.com>
- Fixed summarize/core.py - added cast for sent_tokenize, fixed stop_words type narrowing
- Fixed summarize/freq.py - added cast for sent_tokenize
- Fixed summarize/keybert.py - added type ignore for numpy operations
- Added return type annotation for rank_by_frequency
- Reduced mypy errors from 67 to 56

Co-authored-by: bact <128572+bact@users.noreply.github.com>
- Fixed type annotations for decoded_list in translate files (zh_th, th_fr, small100)
- Removed unused type ignore comments in tokenization_small100.py
- Fixed parse modules to use Optional[str] for model parameters
- Fixed coref/core.py to accept Union[str, list[str]] for texts parameter
- Reduced mypy errors from 56 to 46

Co-authored-by: bact <128572+bact@users.noreply.github.com>
- Added Optional import to parse/esupar_engine.py
- Fixed tokenization_small100.py type ignore comments
- Added type ignore for external library calls in el/core.py and chat/core.py
- Fixed parse/core.py type ignore comment error code
- Reduced mypy errors from 46 to 40

Co-authored-by: bact <128572+bact@users.noreply.github.com>
- Fixed all remaining 40 mypy errors (100% success - 0 errors!)
- Added proper type annotations throughout the codebase
- Fixed import issues in parse and tag modules
- Updated pyproject.toml to ignore ufal module imports
- Applied ruff formatting to all files
- All type hints now properly verified

Co-authored-by: bact <128572+bact@users.noreply.github.com>
- Fixed wordnet.py to use the tokenize parameter as intended
- Fixed thai2fit and ltw2v to properly handle None corpus paths with clear error messages
- Fixed tokenization_small100.py type ignore comment formatting
- All mypy checks still passing (0 errors)

Co-authored-by: bact <128572+bact@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Feb 3, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
14.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@bact
bact marked this pull request as ready for review February 3, 2026 11:48
Copilot AI changed the title [WIP] Verify type hints, annotations, and docstrings Verify and fix type hints, annotations, and docstrings Feb 3, 2026
Copilot AI requested a review from bact February 3, 2026 11:50
@bact
bact merged commit adefafc into dev Feb 3, 2026
30 of 32 checks passed
@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 65.683% (-0.08%) from 65.762%
when pulling 964f209 on copilot/verify-type-hints-and-docs
into bbb5854 on dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring a technical improvement which does not add any new features or change existing features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants